1. I like the new behavior - seems sensible.
2. The patch appears to work with the custom-data imports (no visible errors when installing CiviHR - and the schema/metadata look right in DB).
3. When creating a new "Alphanumeric->Select" field through the web UI in PHP 5.4, it raises an error because $columnName is no longer defined:

    Notice: Undefined variable: columnName in CRM_Core_BAO_CustomField::create() (line 205 of /Applications/MAMP/htdocs/sites/all/modules/civicrm/CRM/Core/BAO/CustomField.php).
    Notice: Undefined variable: columnName in CRM_Core_BAO_CustomField::create() (line 290 of /Applications/MAMP/htdocs/sites/all/modules/civicrm/CRM/Core/BAO/CustomField.php).

The new option-group is consequently misnamed (e.g. "_20130720031447" instead of "myfield_20130720031447").

1. The unit-tests (CRM_Core_BAO_CustomFieldTest, api_v3_CustomFieldTest) regressed. This might be a consequence of the above or might be an issue with the tests relying on the old behavior.

Thanks for the feedback, Tim. I won't be able to investigate issue #3 and
#4 (option-group) till next week.

matt

On Fri, Jul 19, 2013 at 9:21 PM, Tim Otten notifications@github.com wrote:

I like the new behavior - seems sensible.
2.

The patch appears to work with the custom-data imports (no visible
errors when installing CiviHR - and the schema/metadata look right in DB).
3.

When creating a new "Alphanumeric->Select" field through the web UI in
PHP 5.4, it raises an error because $columnName is no longer defined:

Notice: Undefined variable: columnName in CRM_Core_BAO_CustomField::create() (line 205 of /Applications/MAMP/htdocs/sites/all/modules/civicrm/CRM/Core/BAO/CustomField.php).
Notice: Undefined variable: columnName in CRM_Core_BAO_CustomField::create() (line 290 of /Applications/MAMP/htdocs/sites/all/modules/civicrm/CRM/Core/BAO/CustomField.php).

The new option-group is consequently misnamed (e.g. "_20130720031447"
instead of "myfield_20130720031447").

1. The unit-tests (CRM_Core_BAO_CustomFieldTest,
   api_v3_CustomFieldTest) regressed. This might be a consequence of the above
   or might be an issue with the tests relying on the old behavior.

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/1206#issuecomment-21285959
.

@totten Please see the latest patch which should address the issues observed in your testing.

I merged manually and made a couple more changes:

• 79f68e4 - We still need to concatenate the ID# when auto-generating the column name. IIRC, the search system breaks down when columns don't have unique names, and the concatenated ID# protects us from that.
• 43ff319 - Add test-case for the situation in CRM-13089 (ie when the "name" is explicitly set)